Skip to content

[Tooling] Add google owned cache for dependencies as an option in ci #4567

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 18 commits into from
Aug 2, 2023

Conversation

reidbaker
Copy link
Contributor

@reidbaker reidbaker commented Jul 26, 2023

Proof of concept of a package using artifact hub.
Artifact hub is a google owned and managed cache of google/maven dependencies.
Using this cache should decrease the number of flakes related to downloading dependencies along with future benefits of licence analysis and security alerts.

Read more at go/artifact-hub#maven
flutter/flutter/issues/120119

Next steps:
Enable a new env variable on CI servers. https://flutter-review.googlesource.com/c/recipes/+/48260 cl/551888350
Merge this PR.
Apply this change to all packages
Add enforcement as part of the gradle check.

Developers have a new dependency for buildscript but it is publicly available, so I added a next changelog.

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [relevant style guides] and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
  • I signed the [CLA].
  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the [pub versioning philosophy], or this PR is [exempt from version changes].
  • I updated CHANGELOG.md to add a description of the change, [following repository CHANGELOG style].
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • All existing and new tests are passing.

Requried to be an authenticated googler
`gcloud auth application-default login`

Useful links
https://g3doc.corp.google.com/company/teams/artifact-hub/index.md?cl=head#maven
https://github.com/GoogleCloudPlatform/artifact-registry-maven-tools/blob/master/README.md

Useful command
```
gcloud artifacts print-settings mvn \
    --project=artifact-foundry-prod \
    --repository=maven-3p \
    --location=us
```
@reidbaker reidbaker requested a review from camsim99 as a code owner July 26, 2023 15:49
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@reidbaker reidbaker changed the title y [Tooling] Add google owned cache for dependencies as an option in ci Jul 26, 2023
Copy link
Member

@gmackall gmackall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@@ -0,0 +1,16 @@
# Gradle Structure
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok in that case consider the md files content for review and I will remove and update the wiki before merging.

@@ -0,0 +1,16 @@
# Gradle Structure

`package/example/android/settings.gradle` imports the flutter tooling, includes the app directory, and configures GoogleCloudPlatform/artifact-registry-maven-tools for use in ci.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: CI

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


`package/example/android/settings.gradle` imports the flutter tooling, includes the app directory, and configures GoogleCloudPlatform/artifact-registry-maven-tools for use in ci.

This repo has a GCP instance that mirrors dependencies available from `google()` and `mavenCentral()` used by ci (or googlers). This gives us redundant uptime for dependency availability.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nits: CI, Googlers

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


Using this cache is not intended or avaiable for contributors outside of CI. We protect that execution with an environment variable `ARTIFACT_HUB_REPOSITORY` to ensure that by default users do not see rejected cloud credentials or errors in builds.

Googlers can debug locally by setting `ARTIFACT_HUB_REPOSITORY` to the valid artifact hub value and authenticating with GCP. To authenticate run `gcloud auth application-default login`. To find artifact hub url use `<url>` section of go/artifact-hub#maven or inspect the value on CI servers. CI uses a service account for billing. That is defined in go/artifact-hub-service-account (googler access only).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Googler access

@@ -1,3 +1,7 @@
## NEXT

* Updates android example to be able to use custom use artifactregistery.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since nobody outside the Flutter team can use this, let's not put it in the CHANGELOG.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They can if they setup a artifactrepository then setup the env variables on their local machine (I just updated the file to reflect this). I removed the google specific access parts to configuration. Also mechanically there is a new dependency and the changelog thing scolded me.

I can remove this if you want even though there is technically a developer facing change that should not impact the.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stuartmorgan-g stuartmorgan-g added the override: no changelog needed Override the check requiring CHANGELOG updates for most changes label Jul 28, 2023
@stuartmorgan-g
Copy link
Contributor

Overriding changelog check: see #4567 (comment)

@reidbaker
Copy link
Contributor Author

Approved by artifact hub team via email by sfelix@

@reidbaker
Copy link
Contributor Author

@reidbaker reidbaker added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 1, 2023
@reidbaker
Copy link
Contributor Author

Confirmed the environment variable was set correctly https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8773957271362564241/+/u/Run_package_tests/download_Dart_and_Android_deps/execution_details and confirmed the artifact hub variable was used Using artifact hub https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8773957271362564241/+/u/Run_package_tests/download_Dart_and_Android_deps/stdout

https://critique.corp.google.com/cl/552818879 had not landed. Links above show our configuration is correct and that if we lose access to artifact hub our builds will not go red. I do not know of a way to verify we are getting the dependencies from the artifact hub server without ssh access to the machine while they are being downloaded.

In a future pr I could make ci only use the artifact hub instance but I am not confident enough yet to make that call.

@auto-submit auto-submit bot merged commit 11b79b5 into flutter:main Aug 2, 2023
@reidbaker reidbaker deleted the i120119-artifact-hub-poc branch August 3, 2023 14:15
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 3, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 3, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 3, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Aug 3, 2023
flutter/packages@4e4961a...d00c1f9

2023-08-03 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 2.21.1 to 2.21.2 (flutter/packages#4637)
2023-08-03 [email protected] Roll Flutter from b3f99ff to c00d241 (12 revisions) (flutter/packages#4640)
2023-08-03 [email protected] [path_provider] Add getApplicationCachePath() - implementations (flutter/packages#4619)
2023-08-03 [email protected] [various] Removed references to deprecated `TestWindow` APIs (flutter/packages#4558)
2023-08-02 49699333+dependabot[bot]@users.noreply.github.com Bump actions/labeler from 4.1.0 to 4.3.0 (flutter/packages#4432)
2023-08-02 [email protected] [go_router_builder]  Add go_router StatefulShellRoute support to go_router_builder (flutter/packages#4238)
2023-08-02 [email protected] [Tooling] Add google owned cache for dependencies as an option in ci (flutter/packages#4567)
2023-08-02 [email protected] Roll Flutter from 1d59196 to b3f99ff (32 revisions) (flutter/packages#4634)
2023-08-02 [email protected] [go_router_builder] Support `ShellRouteData` without `const` constructor  (flutter/packages#4627)
2023-08-02 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 2.21.0 to 2.21.1 (flutter/packages#4573)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
auto-submit bot pushed a commit that referenced this pull request Aug 7, 2023
- Adds artifact hub check to gradle command
- Add tests for build.gradle and settings.gradle check
- Update all example build.gradle and settings.gradle files

flutter/flutter/issues/120119

Expansion of #4567
zhouyuanbo pushed a commit to zhouyuanbo/video_player_2.7.0 that referenced this pull request Aug 25, 2023
- Adds artifact hub check to gradle command
- Add tests for build.gradle and settings.gradle check
- Update all example build.gradle and settings.gradle files

flutter/flutter/issues/120119

Expansion of flutter/packages#4567
zhouyuanbo pushed a commit to zhouyuanbo/video_player_android_2.4.9 that referenced this pull request Aug 25, 2023
- Adds artifact hub check to gradle command
- Add tests for build.gradle and settings.gradle check
- Update all example build.gradle and settings.gradle files

flutter/flutter/issues/120119

Expansion of flutter/packages#4567
nksteven pushed a commit to nksteven/ez_image_picker that referenced this pull request Aug 29, 2023
- Adds artifact hub check to gradle command
- Add tests for build.gradle and settings.gradle check
- Update all example build.gradle and settings.gradle files

flutter/flutter/issues/120119

Expansion of flutter/packages#4567
tony-hristov pushed a commit to tony-hristov/webview_flutter that referenced this pull request Sep 12, 2023
- Adds artifact hub check to gradle command
- Add tests for build.gradle and settings.gradle check
- Update all example build.gradle and settings.gradle files

flutter/flutter/issues/120119

Expansion of flutter/packages#4567
cbracken pushed a commit to cbracken/flutter_recipes that referenced this pull request May 29, 2024
dko5ki23t pushed a commit to dko5ki23t/google_maps_flutter_improved that referenced this pull request May 24, 2025
- Adds artifact hub check to gradle command
- Add tests for build.gradle and settings.gradle check
- Update all example build.gradle and settings.gradle files

flutter/flutter/issues/120119

Expansion of flutter/packages#4567
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App override: no changelog needed Override the check requiring CHANGELOG updates for most changes p: camera platform-android
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants